Skip to content

feat(reporting): notify the adopter when a reporting issue opens, escalates or resolves - #1875

Merged
bokelley merged 2 commits into
nextfrom
claude/issue-1258-issue-observer
Nov 8, 2026
Merged

bokelley merged 2 commits into
nextfrom
claude/issue-1258-issue-observer

Conversation

@bokelley

@bokelley bokelley commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #1258. A seller can now learn, through a supported callback, when a reporting issue opens, escalates, is acknowledged or waived, or resolves. ESA can drop its get_reporting_status response scan.

from adcp.reporting import ReliableReportingService, ReportingIssueEvent

async def on_issue(event: ReportingIssueEvent) -> None:
    if event.issue is not None and event.issue.responsible_party != "buyer":
        await page(event)  # dedupe on event.event_id

ReliableReportingService(..., issue_observer=on_issue)

Stacked on the storage PR (#1874): until that merges, this diff also shows its commit. Merge #1874 first; this one merges after the adcp-reporting-storage beta release that ships migration 0008. Against a store without the capability, the SDK refuses issue_observer with an actionable error that names adcp-reporting-storage 0.2.0b4.

API

  • issue_observer: ReportingIssueObserver on ReliableReportingService. It is any sync or async callable taking a ReportingIssueEvent.
  • ReportingIssueEvent (frozen) has:
    • event_id: stable across redeliveries.
    • kind: opened | escalated | acknowledged | waived | resolved.
    • occurred_at, account_id, consumer_id, issue_id, attempt.
    • issue: the ReportingIssue as get_reporting_status rendered it for that caller at occurred_at, with responsible_party, recommended_action, severity, opened_at and the causing reporting_status_id. It is None for resolved.
    • reporting_status_id, a convenience accessor.
  • ReportingIssueEventDispatcher for adopters who compose the ledger without the service.
  • Ledger level:
    • ReportingIssueEventStore, the capability protocol, plus supports_issue_events().
    • ReportingIssueEventLease and issue_event_id().
    • The in-memory reference store implements all of it.
  • ReliableReportingTurn.issue_events reports escalations_recorded, delivered and failed.

When it fires

  • After commit, once per transition. The store records each event in the same transaction as its lifecycle write: ingest settle, read settle, set_issue_state, retirement and replay persistence. The record is exactly once across replicas.
  • On the worker turn. run_worker() or worker_interval delivers committed events. The same turn sweeps escalations. It settles the account first, so a cleared condition resolves instead of escalating, and an unattended statement escalates on time without anyone polling.
  • At least once. A crash between the observer returning and the delivery stamp redelivers the event with the same event_id. Deduplicate on it. This is documented in the module docstring, the service docstring and the guide.
  • In order. An occurrence's events arrive in recording order, at most one leased at a time.
  • Never into the buyer's call.
    • An observer that raises, or an async observer past 10 s, is reported to worker_error_handler as issue_observer:{event_id}.
    • The event is retried after a backoff of 30 s doubling to 1 h.
    • A transient snapshot-read failure is retried rather than delivered unrendered.
  • Idempotent. An exact replay of sync_reporting_status (unchanged) and repeated status reads record nothing.

Scope decisions

  • production=. Leaving this out was agreed. The service raises ReliableReportingConfigurationError when production= and issue_observer are passed together. Follow-up: feat(reporting): support issue_observer on the production= service path #1873.
  • Rendering. The issue is re-projected from the account snapshot as of occurred_at: statements recorded by then (chain leaf as of then), obligations and revisions created by then, and the occurrence in the event's state. Revision readability is mutable and has no history, so an event delivered after the seller changed readability can render issue=None. Rendering is documented as best effort.
  • TS parity. @adcp/sdk has no issue-level hook; its closest analog is the obligation-health subscribers/notify outbox. This design mirrors that model: a durable record, at-least-once delivery, a stable id, retry until success, a 10 s timeout. A follow-up asks TS for issueObserver with the same event_id derivation: feat(reporting): issueObserver — notify the seller when a reporting issue opens, escalates or resolves (Python parity) adcp-client#3648.

Tests

  • tests/test_reporting_issue_observer.py (memory):
    • the service flow: open on ingest, one event; replay, status read and idle turn record nothing; escalation at the boundary only, once; resolution on agreement
    • a failing observer: reported, retried with the same event_id, attempt=2
    • per-occurrence ordering behind a failed event
    • timeout of a hung async observer; sync observer
    • operator acknowledged and waived events rendering in their state
    • a cleared condition resolves rather than escalates
    • refusals: production=, an unsupported store, a non-callable observer, a missing table
    • event_id parity with the storage distribution
  • tests/conformance/reporting/test_reporting_issue_observer_pg.py runs the same flows on postgres_storage, plus two replicas delivering each event once and refusal before migration 0008.
  • tests/type_checks/reporting_issue_observer.py (mypy --strict and pyright).

Docs: "Observe reporting issues" in docs/reliable-reporting-service.md, and the worker_error_handler component list.

🤖 Generated with Claude Code

plan: list[Migration] = []
held = False
for migration in pending:
if migration.contract:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium: _past_held_contract (dry-run plan) and _migrate (apply) diverge on a second contract step. _migrate breaks on any non-additive past a held contract (if held and not migration.additive: break), so a second contract step stops the walk. Here a contract step only sets held = True and the loop continues, so an additive after the second contract is planned. For pending [0007c, 0008a, 0009c, 0010a] (all held): _migrate applies [0008], _past_held_contract returns [0008, 0010] — --dry-run/status reports work the real run won't do. Not reachable with today's single contract step (0007), so latent, but the two planners are written to agree and don't. Mirror _migrate: break when held and not migration.additive before testing migration.contract.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — additive issue-lifecycle event outbox + observer under feat(reporting):, correct non-breaking semver signal (new optional issue_observer, new exports, no public-surface removals or type changes).

What I checked

  • New issue_events.py modules, memory store additions (store.py claim/lease/sweep/prune) — exactly-once semantics preserved.
  • persist_replay_lifecycles_on rework: inserted/moved guards avoid double-recording — safe.
  • 0008 SQL: UNIQUE(issue_id,kind) exactly-once, FOR UPDATE SKIP LOCKED + NOT-EXISTS in-order claim — sound.
  • Dispatcher operability: async observer asyncio.wait_for timeout, per-event backoff/release, store failures routed to worker_error_handler.
  • Coverage: memory + PG conformance + type-check tests present.

Medium findings (1)

  • packages/adcp-reporting-storage/src/adcp_reporting_storage/migrations/init.py:349 — Dry-run migration planner diverges from applier on a second contract step (latent; only 0007 exists today).

One medium finding, no critical/high, no escalation triggers (high_risk false, gated_paths false, no team gate, no prior decision). Below the three-medium threshold for comment → falls through to approve.

Medium findings

  • packages/adcp-reporting-storage/src/adcp_reporting_storage/migrations/init.py:349 — Dry-run migration planner diverges from applier on a second contract step

@bokelley
bokelley force-pushed the claude/issue-1258-issue-observer branch from b1aee14 to 43258f8 Compare November 8, 2026 15:39
bokelley and others added 2 commits November 8, 2026 15:41
…sactional outbox

Migration 0008 (header "-- adcp-upgrade: additive") adds
reporting_issue_events and its indexes: one new table, nothing else, and the
storage contract stays 2. Every write that moves a reporting_issue_lifecycle
row (ensure opens, set_issue_state acknowledges or waives, the projection
retires as resolved, replay persistence) records its transition there in the
same transaction, so an event exists exactly when its transition committed.
UNIQUE (issue_id, kind) keeps the record exactly once across replicas, and
event_id is derived from (issue_id, kind), so every redelivery of a
transition carries the same id.

PgReportingLedgerStore gains the outbox the issue observer reads
(adcp-client-python#1258): issue_events_supported and issue_events_ready(),
sweep_issue_escalations(after=...) (settles each candidate account under its
lock, then records 'escalated' for occurrences still open or acknowledged past
opened_at + after, ON CONFLICT DO NOTHING), claim_issue_events (SKIP LOCKED
leases, at most one event per occurrence, each occurrence in recording order),
complete_issue_event, release_issue_event(retry_after=...) and
prune_issue_events(older_than=...), both measured on the store's clock.
ReportingIssueEventLease is exported.

Until 0008 is applied, lifecycle writes record nothing and
issue_events_ready() is False; nothing else changes. python -m adcp migrate
now applies an additive migration even while a contract step waits, so 0008
lands on an expanded ledger beside adcp-reporting-storage 0.1.x/0.2.0b0
processes, which neither read nor write the table. The published-wheel
rolling test runs them beside it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…alates or resolves

ReliableReportingService(issue_observer=...) calls an adopter callback once
per lifecycle transition of a reporting-issue occurrence: opened, escalated,
acknowledged, waived, resolved (#1258). A CONSUMER_STATUS_MISMATCH is
caller-scoped, so until now a seller only learned of one when the buyer read
its status or called; an unattended statement was never noticed.

The ledger records each transition in the same transaction as the lifecycle
write (ingest, status reads, set_issue_state, retirement), exactly once across
replicas. The worker turn sweeps escalations when opened_at +
consumer_mismatch_escalation passes (settling the account first, so a cleared
condition resolves instead of escalating), then delivers committed events to
the observer: after commit, in recording order per occurrence, at least once.
Every delivery of one transition carries the same event_id; observers
deduplicate on it. An observer that raises or times out (10 s) never fails a
buyer's call: it is reported to worker_error_handler as
issue_observer:{event_id} and retried with backoff (30 s doubling to 1 h).

ReportingIssueEvent carries event_id, kind, occurred_at, account_id,
consumer_id, issue_id, attempt and the ReportingIssue as get_reporting_status
rendered it for that caller at occurred_at (None for resolved).
ReportingIssueEventDispatcher serves adopters who compose the ledger without
the service. The in-memory store implements the ReportingIssueEventStore
capability; PostgreSQL needs adcp-reporting-storage 0.2.0b4 with migration
0008, and the service refuses issue_observer with an actionable error
otherwise. production= is refused with issue_observer for now (#1873).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bokelley
bokelley force-pushed the claude/issue-1258-issue-observer branch from 43258f8 to fa4a30c Compare November 8, 2026 15:42

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — additive issue-observer outbox feature (#1258) with no blocking findings.

This is a subsequent review. The prior run flagged one medium finding (migrations/init.py:349 — dry-run planner diverging from applier on a second contract step). That finding is absent from the current run, indicating the developer addressed it. Net improvement.

What was checked:

  • Feature is additive with correct semver and no wire change, so DR-0032/0033 pathway rules don't apply.
  • issue_events.py:327 run_once claims leased batches and settles each lease; :399 _backoff caps retry at retry_cap.
  • ledger/store.py:1921 _record_issue_transition is exactly-once per (issue_id,kind), no-ops on unchanged state; :1973 sweep_issue_escalations settles each account first.
  • ledger/issue_events.py:61 stable dedup id derivation.
  • At-least-once delivery, per-occurrence ordering, failing/hung-observer paths tested incl. two-replica PG conformance.
  • Async observer timeout (10s via wait_for), worker-turn error isolation, escalation idempotency under concurrent sweeps, retention-bounded prune.

Gates: gated_paths is false; review_decision is APPROVED; no no-auto-approve team match; high_risk false.

Decision table: no critical/high/medium findings present (rows 1, 4–8 do not fire), gated_paths false (row 2 N/A), no deletions in high-risk paths (row 3 N/A). Falls through to row 9 → approve.

Non-blocking follow-up noted by reviewer: _snapshots adds a sibling resolved lease to unreadable on transient snapshot-read failure — a bounded, self-healing delay on the all-clear path, not data loss. Worth a glance but not actionable.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Approve

Approve — additive issue-lifecycle event outbox and observer (#1258), purely additive public surface under feat: with the correct semver signal.

Gates checked:

  • No no-auto-approve team match; gated_paths: false; review_decision: APPROVED.
  • high_risk: false.
  • No critical/high findings; no current medium findings.

Subsequent-review note: the prior Medium (dry-run migration planner diverging from the applier at migrations/__init__.py:349) is resolved — _past_held_contract now mirrors _migrate's held-contract walk, with a test pinning the second-contract-step case. Net improvement over the prior pass.

Reviewer verified: additive public surface (no wire/union/layering/credential concerns), CI gates intact, observer is a seller-side local callback (not a wire change, so DR-0032/0033 pathways don't apply), dispatcher lease sizing and lease_token/ON CONFLICT guards give correct at-least-once delivery, claim query enforces one-in-flight-per-occurrence across replicas, and the escalation sweep settles before escalating.

No blocking or medium findings remain — falls through to row 9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant